Support colocated joins when a partition holds no segments - #19166
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #19166 +/- ##
=============================================
+ Coverage 0.00% 67.17% +67.17%
- Complexity 0 1424 +1424
=============================================
Files 3 3462 +3459
Lines 6 220358 +220352
Branches 0 35146 +35146
=============================================
+ Hits 0 148030 +148030
- Misses 6 60509 +60503
- Partials 0 11819 +11819
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
151db80 to
5a48ddb
Compare
There was a problem hiding this comment.
Pull request overview
Enables colocated joins to handle empty partitions while preserving partition-to-worker alignment and reducing fan-out.
Changes:
- Adds shared partition-class assignment and empty-worker padding.
- Adds deferred-partition and multi-cluster safeguards.
- Expands planner, broker, mailbox, and integration coverage.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pinot-query-planner/.../WorkerManagerTest.java |
Covers partition assignment and padding. |
pinot-query-planner/.../ColocationGroupAnalyzerTest.java |
Tests reducible group detection. |
pinot-query-planner/.../QueryEnvironmentTestBase.java |
Updates partition metadata construction. |
pinot-query-planner/.../PinotDispatchPlannerTest.java |
Tests spooled-stage assignment. |
pinot-query-planner/.../MailboxAssignmentVisitorTest.java |
Tests direct-exchange class agreement. |
pinot-query-planner/.../DispatchableSubPlanTest.java |
Tests segment-map preservation. |
pinot-query-planner/.../WorkerManager.java |
Implements class reduction and padding. |
pinot-query-planner/.../LeafPartitionHints.java |
Centralizes partition-hint parsing. |
pinot-query-planner/.../ColocationGroupAnalyzer.java |
Identifies colocated fragment groups. |
pinot-query-planner/.../MailboxAssignmentVisitor.java |
Validates direct-exchange mappings. |
pinot-query-planner/.../DispatchablePlanMetadata.java |
Stores broker-local class metadata. |
pinot-query-planner/.../DispatchablePlanFragment.java |
Preserves segment assignments when copied. |
pinot-query-planner/.../DispatchablePlanContext.java |
Caches partition metadata per query. |
pinot-integration-tests/.../ColocatedJoinEmptyPartitionTest.java |
Adds end-to-end join coverage. |
pinot-core/.../TablePartitionReplicatedServersInfo.java |
Publishes deferred partitions. |
pinot-broker/.../SegmentPartitionMetadataManagerTest.java |
Tests deferred-partition detection. |
pinot-broker/.../MultiClusterRoutingManagerTest.java |
Tests cross-cluster partition metadata. |
pinot-broker/.../SegmentPartitionMetadataManager.java |
Tracks deferred-only partitions. |
pinot-broker/.../MultiClusterRoutingManager.java |
Avoids combining partition arrays. |
Suppressed comments (2)
pinot-broker/src/main/java/org/apache/pinot/broker/routing/manager/MultiClusterRoutingManager.java:239
- A failed remote lookup leaves it unknown whether that cluster also serves the table. Continuing can return another cluster's partition array and enable a partition-aware plan over a partial view, which can silently drop remote-only partitions. Return
nullon this failure so the caller falls back instead.
} catch (Exception e) {
LOGGER.error("Error getting table partition info from remote cluster routing manager for table {}",
tableNameWithType, e);
continue;
pinot-broker/src/main/java/org/apache/pinot/broker/routing/manager/MultiClusterRoutingManager.java:243
- A
nullpartition-info response does not mean this cluster lacks the table; a routing entry can exist without usable partition metadata. Treating that cluster as absent can return the local array for a table spread across clusters and silently omit its partitions. CheckroutingExistsand returnnullwhen the remote cluster serves the table.
if (remotePartitionInfo == null) {
continue;
}
| partitionsWithOnlyDeferredSegments.removeIf( | ||
| partitionId -> partitionId < partitionInfoMap.length && partitionInfoMap[partitionId] != null); |
| TablePartitionReplicatedServersInfo partitionInfo = | ||
| _localClusterRoutingManager.getTablePartitionReplicatedServersInfo(tableNameWithType); |
5a48ddb to
45447e7
Compare
A colocated join failed outright when a partition of one of its tables held no segments, with "Failed to find any segment for table: X, partition: N". Worker ids came from a running counter over the partitions that held data, so skipping an empty one would shift every later partition down a slot. Two tables each dropping a different empty partition could then end up with equal worker counts and be wired 1-to-1 onto mismatched partitions, losing rows with no error, which is why the assignment refused to continue at all. The stages tied together by direct exchanges now share one ordered list of partition classes, dropping only the classes that hold no data on any member. A class the group keeps but a member holds no data for gets a worker with no segments, placed on a server borrowed from a member that does hold that class so the exchange stays in process. The two sides of every direct exchange assert that they agree on the list. The broker publishes the partitions whose only segments are new and have no online replica. Those hold data that no server can serve as a whole, so they keep failing rather than being read as empty. A worker with no segments is charged against the query thread estimate like any other worker: it is dispatched and does run a leaf operator. The estimate therefore over-counts by two threads per such worker, which is conservative and only affects colocated joins over a partition space that is largely unpopulated. Those queries failed outright before, so there is no earlier estimate to compare against. Aggregation merge identity is deliberately not covered here. A leaf that scans nothing emits one identity row for an aggregation with no GROUP BY, but that predates this change: a worker whose segments are all pruned on the server already does the same. Padding raises how many such rows reach the merge without introducing the dependency, and the one aggregation that is not a true merge identity fails only when every worker is empty, which this change does not newly reach.
39da748 to
5526d81
Compare
## Summary - document colocated joins when one table has an empty partition - explain shared partition-class alignment and reduced worker fan-out - call out partition-metadata validation and remaining failure cases ## Upstream context Follows apache/pinot#19166. ## Validation - `git diff --check` Co-authored-by: Xiang Fu <xiangfu@Xiang-mac-mtv-2.local>
|
The follow-up documentation is available in pinot-contrib/pinot-docs#996: pinot-contrib/pinot-docs#996 |
Why
A colocated join fails today when a partition of one of its tables holds no segments:
This is easy to hit and hard to avoid:
The assignment cannot simply skip the empty partition. Worker ids come from a counter over the partitions that hold data, so skipping one shifts every later partition down a slot. Two tables that each drop a different empty partition then end up with equal worker counts, get wired one-to-one, and join mismatched partitions. That loses rows and reports no error, which is why the assignment refuses to continue at all.
What changes
A side effect is less fan-out. A table that declares 8 partitions and populates 3 runs 3 workers, so the broker sends fewer requests and waits on fewer servers.
What now fails on purpose
Broker pruning comes next
Broker pruning is off for a colocated join today, and this change is what makes it possible.
A follow-up PR adds it.
Testing
Three things left out on purpose
Aggregation merge identity is not covered here. A leaf that scans nothing emits one identity row for an aggregation with no
GROUP BY. That predates this change: a worker whose segments are all pruned on the server already does the same. This change raises how many such rows reach the merge without introducing the dependency. The one aggregation that is not a true merge identity,PERCENTILEKLL, fails only when every worker is empty, which this change does not newly reach. It is tracked separately.A worker with no segments is charged against the query thread estimate like any other worker. It is dispatched and does run a leaf operator. The estimate over-counts by two threads per such worker. That is conservative, and it only affects colocated joins over a partition space that is largely unpopulated. Those queries failed outright before, so there is no earlier estimate to compare against.
A partitioned table outside a colocated join still fails on an empty partition. A shuffled send puts no constraint on its worker ids, so the leaf joins no group and gets no class list. The error is the same one this change removes for colocated joins, and it predates this change.
Labels
Applied:
bug,backward-incompat,release-notes,multi-stage,query